Skip to content

#27 - Provide cleanup-after-each #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 1, 2019
Merged

#27 - Provide cleanup-after-each #31

merged 4 commits into from
Jun 1, 2019

Conversation

dfcook
Copy link
Collaborator

@dfcook dfcook commented Jun 1, 2019

  1. Add cleanup-after-each in same manner as react-testing-library
  2. Add jest module name mapper so tests can import the scoped package
  3. Update package name in readme

afontcu
afontcu reviewed Jun 1, 2019
View reviewed changes
Copy link
Member

@afontcu afontcu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice! Changing npm/ to dist/ makes total sense 👍

Yet, I can't see the cleanup-after-each file in the PR 🤔

oh, and should we import it to each demo test files? I noticed some test files are cleaning up the DOM and some aren't, it'd be nice to clean it up everywhere (as I can't think of any use case where you'd want the DOM to be dirty!)

@dfcook
Copy link
Collaborator Author

dfcook commented Jun 1, 2019

Not sure how that file got missed, I've added it now.

I've added cleanup to each test file that had > 1 test. Looking at how the cleanup-after-each is implemented it specifically references dist so not sure it would be runnable from the tests.

@afontcu
Copy link
Member

afontcu commented Jun 1, 2019

Yes, you are right 👍 Just checked react-testing-library and they also import and add afterEach(cleanup) in their demos (example). It's OK I think, the cleanup-after-each file is a nice addition but the important part is cleaning up the DOM so tests don't mess with each other.

@dfcook dfcook merged commit 1367d61 into master Jun 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afontcu afontcu afontcu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dfcook @afontcu